Repository navigation
fix(keyboard): forward unbound Cmd+Shift combinations to terminal - #1757
BillionClaw wants to merge 1 commit into
Conversation
Unbound Cmd+Shift+<key> combinations were being silently swallowed by the AppKit key-routing layer. The two-pass timestamp mechanism in performKeyEquivalent relied on AppKit to redispatch events, but AppKit never redispatches unbound keys that don't match menu items. Fix by forwarding unbound Command-modified keys directly to keyDown instead of returning false and hoping for redispatch. This ensures the kitty keyboard protocol receives the events. Fixes manaflow-ai#1718
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThe fix addresses a bug where unbound Cmd+Shift key combinations were silently consumed by AppKit's menu routing instead of reaching the terminal. The code now explicitly forwards unbound Cmd-modified keys to the terminal via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4881-4888: The new unbound-Command forwarding path in
GhosttyTerminalView.swift unconditionally clears lastPerformKeyEvent and calls
keyDown(with: event), which incorrectly forwards Cmd+` (keyCode 50) and breaks
AppKit/window-menu routing; update the handler to detect the Command-modified
backtick (check event.keyCode == 50 and event.modifierFlags contains .command)
and skip the forwarding for that combo (leave lastPerformKeyEvent intact and
allow normal AppKit/menu routing), while continuing to forward other unbound
Command keys via lastPerformKeyEvent = nil; keyDown(with: event).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ce0990c-92ac-4823-bef4-1fbca4309d32
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
| // For unbound Command-modified keys (e.g., Cmd+Shift+K with no binding), | ||
| // forward directly to the terminal rather than relying on AppKit's redispatch. | ||
| // AppKit may drop the event if no menu item matches, preventing the key from | ||
| // reaching the terminal via the kitty keyboard protocol. | ||
| // See: https://github.com/manaflow-ai/cmux/issues/1718 | ||
| lastPerformKeyEvent = nil | ||
| keyDown(with: event) | ||
| return true |
There was a problem hiding this comment.
Exclude `Cmd+`` from the new unbound-Command forwarding path.
Line 4881 currently forwards all unbound Command keys to keyDown(with:). That also catches `Cmd+`` (keyCode 50), which should remain on AppKit/window-menu routing for window cycling.
🔧 Proposed fix
- lastPerformKeyEvent = nil
- keyDown(with: event)
- return true
+ // Keep Cmd+` on AppKit/window-menu routing (window cycling).
+ if event.keyCode == 50 {
+ lastPerformKeyEvent = nil
+ return false
+ }
+
+ lastPerformKeyEvent = nil
+ keyDown(with: event)
+ return trueBased on learnings Repo: manaflow-ai/cmux — Command-backtick (Cmd+, keyCode 50) is intentionally excluded from direct menu routing; do not add a bypass for Cmd+.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/GhosttyTerminalView.swift` around lines 4881 - 4888, The new
unbound-Command forwarding path in GhosttyTerminalView.swift unconditionally
clears lastPerformKeyEvent and calls keyDown(with: event), which incorrectly
forwards Cmd+` (keyCode 50) and breaks AppKit/window-menu routing; update the
handler to detect the Command-modified backtick (check event.keyCode == 50 and
event.modifierFlags contains .command) and skip the forwarding for that combo
(leave lastPerformKeyEvent intact and allow normal AppKit/menu routing), while
continuing to forward other unbound Command keys via lastPerformKeyEvent = nil;
keyDown(with: event).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:4888">
P2: Unbound Command key equivalents are now always consumed in `performKeyEquivalent`, which can block app/menu shortcuts from routing through AppKit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // See: https://github.com/manaflow-ai/cmux/issues/1718 | ||
| lastPerformKeyEvent = nil | ||
| keyDown(with: event) | ||
| return true |
There was a problem hiding this comment.
P2: Unbound Command key equivalents are now always consumed in performKeyEquivalent, which can block app/menu shortcuts from routing through AppKit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 4888:
<comment>Unbound Command key equivalents are now always consumed in `performKeyEquivalent`, which can block app/menu shortcuts from routing through AppKit.</comment>
<file context>
@@ -4878,18 +4878,17 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ // See: https://github.com/manaflow-ai/cmux/issues/1718
+ lastPerformKeyEvent = nil
+ keyDown(with: event)
+ return true
}
</file context>
|
Closing older duplicate PR; keeping the newest open. |
1 similar comment
|
Closing older duplicate PR; keeping the newest open. |
|
I’ll get the CLA signed — will follow up once it’s done. |
|
Closing per repository blocklist: maintainer threatened to ban. All submissions to this repo have been suspended. |
2 participants
Fixes #1718
Problem
Unbound
Cmd+Shift+\<key\>key combinations were being silently swallowed by the AppKit key-routing layer. They never reached the terminal application via the kitty keyboard protocol.The root cause was in
performKeyEquivalent: the two-pass timestamp mechanism returnedfalseon the first pass, expecting AppKit to redispatch. However, AppKit never redispatches events that don't match any menu item or Ghostty binding.Solution
Forward unbound Command-modified keys directly to
keyDowninstead of relying on AppKit's redispatch mechanism. This ensures the kitty keyboard protocol receives the events for applications like Neovim that rely onsupermodifier keys.Changes
GhosttyTerminalView.swiftperformKeyEquivalent methodTesting
Cmd+Shift+Kshould now reach the terminal as CSI escape sequence\<S-D-J\>/\<S-D-K\>mappingsSummary by cubic
Forward unbound Cmd+Shift key combinations directly to the terminal so they are no longer dropped by AppKit. Fixes missing key events in apps using the kitty keyboard protocol (e.g., Neovim).
keyDowninperformKeyEquivalent.Written for commit f0f297a. Summary will update on new commits.
Summary by CodeRabbit
Release Notes